Skip to content

Conversation

@sunshowers
Copy link
Contributor

@sunshowers sunshowers commented Oct 16, 2025

Ensure that DNS lookups for all internal services work on the example system, through both the standard and qorb DNS resolvers.

I did notice that the example system doesn't set up every service. Something worth doing in a followup, but this does catch issues like (manually tested) the repo depot response being too large.

Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
Copy link
Collaborator

@davepacheco davepacheco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for doing this!


out.insert(ServiceName::CruciblePantry, Ok(()));

// XXX the SledAgent service name doesn't appear to be used?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it probably is unused but arguably should be. I think we could incorporate it in the same way that you did for Crucible below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note we don't even register the sled agent service names within blueprint_internal_dns_config today. Have added these to the map with a response of QueryError::NoRecordsFound.

);
}

// TODO: add assertions on the returned addresses.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// TODO: add assertions on the returned addresses.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we should check that the addresses match what we expect? I kind of skipped over that because I wanted to detect packet fragmentation first.

Created using spr 1.3.6-beta.1
Copy link
Collaborator

@davepacheco davepacheco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thanks for this.

| ServiceName::CruciblePantry => {
out.insert(service, Ok(()));
}
// Services that are not currently part of the example system.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(sorry if we already did this)
Can you file an issue for including these in the example system?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created using spr 1.3.6-beta.1
@sunshowers sunshowers enabled auto-merge (squash) November 5, 2025 22:30
Created using spr 1.3.6-beta.1
@sunshowers sunshowers merged commit 602fdb5 into main Nov 6, 2025
17 checks passed
@sunshowers sunshowers deleted the sunshowers/spr/reconfigurator-planning-add-test-to-ensure-dns-lookups-work-on-example-system branch November 6, 2025 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants